Skip to content

Conversation

@gierens
Copy link
Member

@gierens gierens commented Aug 20, 2023

Since it took me a moment to notice and grasp it myself how commit messages were altered on my PRs, I though I'd add some guidelines to CONTRIBUTING.md so contributors already try to use the wanted format and maintainers don't have to edit every single commit.

Feel free to adjust it! I was just guessing these from my experience.

PThorpe92
PThorpe92 previously approved these changes Aug 20, 2023
Copy link
Member

@PThorpe92 PThorpe92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect 👍 that document was a little bare. I had just written something up yesterday too in my local repo, so I just made it into a gist.

Maybe either add a link to that
https://gist.github.com/PThorpe92/103cdf8201ae5cd6ee1d426a71010c83#file-eza_commits-md

or honestly you covered everything there so you could just link to
https://www.conventionalcommits.org/en/v1.0.0/

somewhere in there just to specify that we use the conventional commits standard.

@gierens
Copy link
Member Author

gierens commented Aug 20, 2023

Ah, thank's for the hints ... but now I'm actually wondering, if I'm not reinventing the wheel, since https://www.conventionalcommits.org/en/v1.0.0/ covers this in much more detail. so maybe this link and an example would be enough?

Another question, do we actually want to link v1.0.0 ... or better https://www.conventionalcommits.org/en/ since it automatically links to the most recent version.

@PThorpe92
Copy link
Member

Another question, do we actually want to link v1.0.0 ... or better https://www.conventionalcommits.org/en/ since it automatically links to the most current version.

Good catch, that's the way to go.

so maybe this link and an example would be enough?

Personally I like seeing a nicely formatted markdown document with examples and a good description of what projects want their commit messages to look like.

The link is important for sure for the whole specification, but if you look at the gist I made, to see examples of specific changes and how they would relate to the project is nice to have.

build: Changes that affect the build system or external dependencies (example libgit2)

It's up to everyone, but even just adding the link to what you have already would work just fine.

@gierens
Copy link
Member Author

gierens commented Aug 21, 2023

Yeah, you are right I like that as well ... better than having to read specifications or skim recent contributions to get a clue. I will have a look at everything again, the link, your suggestion, my suggestion, and try to come up with a short yet precise description of what is desired from our side with special focus on the type definitions and project related examples.

@cafkafk
Copy link
Member

cafkafk commented Aug 21, 2023

Agree on using https://www.conventionalcommits.org/en/ so we're automatically
up to date.

I want to hear what both of you, as contributors, find the most useful, but my
thought were that having perhaps a combination of both of your writeups, as
well as a link to the full conventional commits specification would be the most
useful.

That way, we have a shorter digestible version from people that have already
gone through the process, as well as a general reference to refer to for more
complicated questions.

Also, one thing I would like to note that @mentalisttraceur made me aware of.

#117 (comment)

The accessibility concern is very valid thou, I guess we could require the
BREAKING CHANGE: footer if ! is present, perhaps we might get someone with a
screen reader to tell us how annoying the ! is, and only use the footer.

I think beyond just screen readers, it would just make sense if we made a
general guideline that changes that are breaking should specify that footer, as
well as a description of what was broken. E.g. BREAKING CHANGE: Env vars EXA_<> now EZA_<>

Also, for the part about "trailers" (as I found out they are called just now),
we might also consider linking to
https://git-scm.com/docs/git-interpret-trailers#_examples.

@mentalisttraceur
Copy link
Contributor

mentalisttraceur commented Aug 21, 2023

Re: "should we link v1.0.0 or the most recent version"

I would specify v1.0.0 - same reasoning as for pinning versions of dependencies. So that

  1. Every commit follows the same spec, until the project consciously decides it wants to bring in future spec changes (which might be potentially anything).

  2. People/tools who look at the commit messages of this repo can trust that they follow a single spec, rather than "whatever version of the spec was live when this commit was made" (or even "whatever version of the spec was live when the committer last checked if there was a new version of the spec").

[More thoughts on Conventional Commits]

  1. Conventional Commits really ought to have documented its own versioning scheme. For example, if they add "feature" as an alias for "feat", will they do that as v1.1.0 because it doesn't make old commits non-conforming, or as v2.0.0 because it would add something that old parsers can't handle, or would they consider themselves free to make it v1.0.1 if someone convinced them enough that it was a design bug to have ever not supported it? (Ever since SemVer versioned itself with SemVer-resembling versions, it's become fashionable to slap looks-like-SemVer strings on specs themselves, totally missing the main insight of SemVer that version strings are next to useless unless you actually take the time to specify what they mean.)

  2. Conventional Commits really ought to have links on its website for going to the latest version which

    1. does not remove the ability to express the same things the same way as you could in [version], and
    2. does not add anything that parsers of [version] can't understand.

    Then a project like Eza could decide which of those two properties it wants, and link to that (this is analogous to specifying acceptable versions of SemVer dependencies as v1.* or v1.2.*).

@mentalisttraceur
Copy link
Contributor

mentalisttraceur commented Aug 21, 2023

Conventional Commits also really ought to provide a standard way of declaring+detecting what version of the spec is being followed in a repo for any given commit. After thinking through a few designs, here's the standard I would propose (posting it here for initial feedback, but I'll probably post it as a proposal in the Conventional Commits repo soon):

The project adds a COMMITS.md file in the root of the repo, which must start with exactly the line https://www.conventionalcommits.org/{{language}}/{{version}}, where {{language}} is the language identifier (ignored by tooling - a tolerance to let projects use the link to their native language), and {{version}} is for example v1.0.0 (optionally with a trailing /). Nothing else on that first line.

(In a precise spec I'd probably mandate that this string must be in UTF-8/ASCII, to reduce the odds that one popular implementation goes looser and all tools end up having to know how to detect or interpret other byte patterns.)

(For a proper spec, note that language identifiers are not always just two characters like en, but sometimes it's strings like pt-br, and a spec should either pin down precisely what it could be, or go explicitly loose and permit one URL path part, no matter what it is.)

Advantages/rationale:

Both tools and humans can know if Conventional Commit semantics apply to a commit, and which exact version. We can even know this with per-commit granularity by checking just the start of that file's contents for a given commit (for those who don't know, something like git show can do that without having to check out the whole commit) - and we get this per-commit granularity without having to specify the spec version in every commit. With that solved, the spec is much more free to evolve semantics without breaking interpretation by tooling.

By keeping it super simple (the version string and the URL string are one, it's the very first thing in the file by itself on the first line, there is no markdown syntax to parse, etc), we make it extremely simple+easy for tools to know what version of the spec is being used, humans can still see and understand a URL (and the markdown extension helps editors/viewers automatically style it as a link, make it clickable, etc), and there's a clear single source of truth.

By making the spec explicitly about the first line (so tools just look at the first line and ignore/tolerate the rest), it allows forward-compatibility with future spec changes or other unforeseen uses adding other information.

Discoverability is really high because it looks like one of the conventional about-this-repo files (all-uppercase, like README, CONTRIBUTING, etc), it points you right to the full documentation of that commit convention. It would also sort in basically the same place as a CONTRIBUTING file in an alphabetic listing - if I was looking for a "contributing" file and saw a "committing" file (as a dev who already knows what a "commit" is) I'd almost certainly think to look in it.

@cafkafk
Copy link
Member

cafkafk commented Aug 22, 2023

Moving to #160, as it's not something we'd need to consider for this pull request.

I want to hear what both of you, as contributors, find the most useful, but my
thought were that having perhaps a combination of both of your writeups, as
well as a link to the full conventional commits specification would be the most
useful [for now].

Signed-off-by: Christina Sørensen <[email protected]>
Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't perfect, but there's no reason to wait merging this until it is. It's better to have something than nothing, specially so I can direct new contributors to it.

@cafkafk cafkafk merged commit 4fe03a2 into eza-community:main Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants